Skip to content

Make kyuubi.engine.security parameters public#7418

Open
aajisaka wants to merge 3 commits intoapache:masterfrom
aajisaka:public-engine-secure
Open

Make kyuubi.engine.security parameters public#7418
aajisaka wants to merge 3 commits intoapache:masterfrom
aajisaka:public-engine-secure

Conversation

@aajisaka
Copy link
Copy Markdown
Member

Why are the changes needed?

kyuubi.engine.security.enabled and related parameters are not in preview or internal feature. Make them public.

Discussion: https://lists.apache.org/thread/5nvbt0kwlfd9v3tvs3f9d4tfjd25rnb9

How was this patch tested?

Doc only change. Testing is not required.

Was this patch authored or co-authored using generative AI tooling?

No

@github-actions github-actions Bot added kind:documentation Documentation is a feature! module:common labels Apr 24, 2026
@pan3793
Copy link
Copy Markdown
Member

pan3793 commented Apr 24, 2026

I remember this config name causes some ambiguity, and some features are enforced to only be available when enabling this config ... we indeed need to document that, I will likely loop this part of code this weekend or early next week

@aajisaka
Copy link
Copy Markdown
Member Author

Thanks. Documented that the features are available only when kyuubi.engine.security.enabled is set to true. Also enhanced the doc for kyuubi.engine.security.secret.provider.

@pan3793
Copy link
Copy Markdown
Member

pan3793 commented Apr 30, 2026

@aajisaka I recall the background. The security feature is requested and implemented by @turboFei from eBay.

At the beginning, Kyuubi only supports interactive mode, there is no communication between Kyuubi server instances, so security applies to server <=> engine communication, and the config is named as kyuubi.engine.security.enabled.

Later, Kyuubi added support for batch mode, and the HA implementation contains the HTTP request between Kyuubi server instances, it reuses kyuubi.engine.security.enabled config, and expands the scope to all internal communications - both server <=> server and server <=> engine.

What's confusing?

  1. the config name, obviously, kyuubi.engine.security.* is misleading, we might change it to kyuubi.internal.security.* and add a fallback to avoid breaking changes.
  2. kyuubi.engine.security.enabled must be set to true to enable batch HA, this was strongly insisted on by the feature author @turboFei, for security reasons. But I think we should remove such a requirement. (as a workaround, I introduced SimpleEngineSecuritySecretProviderImpl to allow admin to configure pre-share secret key on kyuubi-defaults.conf, to avoid introducing a zk dependency to store the secret key). This should be documented and simplified, otherwise, users are not able to enable the batch HA without looking at the code in depth.

What should we do?

  1. rename the config to kyuubi.internal.security.* with a fallback, and make those configs publicly.
  2. make a decision on whether we can remove the internal security-enabled requirement for batch HA.
  3. write a document to briefly explain how internal security works, how to enable/configure it, and how to extend it (e.g., how to implement a custom secret provider)

@aajisaka
Copy link
Copy Markdown
Member Author

aajisaka commented May 1, 2026

Thank you @pan3793 for the detailed context.

Agreed to rename the config. Renamed them in the latest commit.

make a decision on whether we can remove the internal security-enabled requirement for batch HA.

Before removing the requirement, we need to ask admin to secretly store the pre-share secret key. It must not be visible to users. Probably admin are required to configure kyuubi.server.redaction.regex as well. Is it sufficient, or do you have any idea? My preference is always redact the secret key regardless of the regex config.

write a document to briefly explain how internal security works, how to enable/configure it, and how to extend it (e.g., how to implement a custom secret provider)

Totally agreed. I'll write a doc later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind:documentation Documentation is a feature! module:common

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants